fix(workflow): drain the queue when a load-time reap ends a run - #2757

…ThinkFlowLab#2773)

Fixes ThinkFlowLab#2748

Owner: Dev-3
Reviewer: Birfy
Written-by: Dev-3
Bot: Dev-3
Claim-Id: 9df438ed10ac478b8d037965974377ec
Base: f2ef4df
Risk tier: T2

## The defect

The step deadline is the one thing that bounds every wait in a live run,
and `expireStep()` ran the whole expiry inside `hostSerial`. The serial
tail is exactly where a host call that never answers sits: `dispatch`,
`release` and `spawn` are each awaited there. So a host that never
answered parked the tail for ever, the timer that ends the run fired and
queued behind it, and the run stayed `running` with its Bots still held
and its lane slot never freed. A Bot start was worse off: `takeNext()`
had already shifted the card out of `lane.queue`, so a mute host left
the card neither running nor queued — it was simply gone.

## What changed

- `plugins/workflow/src/service.js` — `expireStep()` no longer runs
inside `hostSerial` (`return hostSerial(async () => {…})` becomes
`return (async () => {…})()`), the one-line shape the issue names; the
body still ends with `await drainAll()`. A deadline can no longer be
queued behind the wait it bounds, so a dispatch or a release that never
answers still ends the run `failed`, releases its Bots and clears its
timers.
- `SPAWN_DEADLINE_MS` (60 s) and `boundedSpawn()`: a start whose Bots
the host does not create within that bound is refused with `code:
'spawn-timeout'`, and a late answer's Bots are released instead of
leaked. `startLive()` wraps `spawnAll(…)` in it, so `drainAll()` takes
the existing `started.ok === false` path and unshifts the card it had
taken back to `lane.queue`.
- `plugins/workflow/test/tail-hang.test.js` (new) — three probes on a
fake clock: C1 a dispatch that never settles, C2 another workflow's
release that never settles, C4 a spawn that never answers. C1/C2 assert
the run and node end `failed` with Bots released and no timer left; C4
asserts the call answers, the queue still holds the card behind it, and
the lane is idle.
- `plugins/workflow/src/service.js` — a start that is still waiting on
the host now counts against its lane's `maxConcurrent` (Test's advisory
on this PR): two steps ending in the same turn each run `drainAll()`,
the first start is parked on the host and `takeNext()` counts only begun
runs, so the second drain takes a second card and the lane reaches
`activeCount 2` at `maxConcurrent 1`. The count is `startsInFlight`, one
number per workflow, never persisted.
- `plugins/workflow/test/cap-concurrent-drain.test.js` (new) — that
case: lane cap 1, a live run plus two queued cards, both deadlines in
the same turn, every spawn held. It asserts the lane never passes
`maxConcurrent`, the card the cap kept out stays in the queue, and it
starts once the slot frees.
- `docs/subsystems/workflow.md` — the deadline paragraph now states that
the expiry is not queued behind the wait it bounds, and names the 60 s
start bound; the test list names the three hangs.
- `changelog.d/2748-workflow-stuck-run.fixed.md` — the user-visible
entry.

## The race the advisory found, and the fix

Removing `hostSerial` from `expireStep()` let two drains reach one lane
in the same turn, and `takeNext()` counts only runs `beginRun()` has
already made visible. The first drain was still parked in `await
boundedSpawn(spawnAll(…))` when the second looked for a card, so the
second read the lane as empty and started a card too. Probe reading at
`cc35bf48`, 6/6:

```
# PROBE S3 cap under concurrent drain: FAIL — activeCount=2 (3 live run(s): #S3b,#S3c,#S3a) at maxConcurrent 1
```

The fix counts a start in flight: `takeNext()` and `hasRoom()` add
`startsOf(workflow)` to `runningOf(lane).length`, and `startLive()`
gives that count back the moment `beginRun()` puts the run where
`runningOf()` sees it — and on every way out that never gets that far (a
refused start, a spawn timeout, a missing provider). Holding it through
the hand-off instead would count a slot nothing is using, and a dispatch
that never answers would hold it for good — so the release sits at
`beginRun()`, not after `startLive()` returns.

## Red first

At the live base `f2ef4dfa` with only the test file added, twice in a
row with the same shape — `# tests 3 / # pass 0 / # fail 3`:

```
# PROBE C1 status: running node: running active: 1 released: 0 timersLeft: 0
not ok 1 - ThinkFlowLab#2748 C1: a dispatch that never settles still reaches failed at its step deadline
  the deadline ends the run even while the dispatch hangs
  + 'running' - 'failed'
  TestContext.<anonymous> (tail-hang.test.js:116:10)

# PROBE C2 alpha: running bravo before: running after its deadline: running bravo node: running released: 0 timersLeft: 0
not ok 2 - ThinkFlowLab#2748 C2: a release that never settles does not starve another workflow's deadline
  the other workflow still reaches its deadline
  + 'running' - 'failed'
  TestContext.<anonymous> (tail-hang.test.js:165:10)

# PROBE C4 settled: false queue: 0 active: 0 card: none
not ok 3 - ThinkFlowLab#2748 C4: a spawn that never answers keeps the queued card instead of losing it
  the call answers instead of hanging for ever
   false !== true
  TestContext.<anonymous> (tail-hang.test.js:201:10)
```

The base probes say the whole bug in one line each: C1 and C2 keep the
run `running` with `released: 0`, and C4's card is `none` — neither
running nor queued.

The race has its own red at `cc35bf48` — only
`cap-concurrent-drain.test.js` added, twice with the same shape, `#
tests 1 / # pass 0 / # fail 1`:

```
not ok 1 - ThinkFlowLab#2748 S3: two drains that reach one lane in the same turn keep it under maxConcurrent
  one run at maxConcurrent 1, not 2
  TestContext.<anonymous> (cap-concurrent-drain.test.js:92:1)
```

## Verification at the tip

Tip SHA: `a79976fddd7599ab32b0615c0595aea051305820` (tree
`ffa6635fd9224e38363cf1faa59e4b22f8e113b6`)

Ran:

- `cd plugins/workflow && node --import ./test/resolve.mjs --test
test/tail-hang.test.js` → `# tests 3 / # pass 3 / # fail 0`, twice in a
row, same shape, with the PROBE lines C1 `status: failed node: failed
active: 0 released: 2 timersLeft: 0`, C2 `alpha: failed bravo before:
running after its deadline: failed bravo node: failed released: 2
timersLeft: 0`, C4 `settled: true queue: 1 active: 0 card: ThinkFlowLab#2748-C4b`.
- `cd plugins/workflow &&
DSH_HARNESS=/home/xuke/.local/lib/node_modules/@deepseek-ai/dsh node
--import ./test/resolve.mjs --test "test/*.test.js"` → `# tests 512 / #
pass 512 / # fail 0` (511 before this commit, plus the new case).
- `cd plugins/workflow && node --import ./test/resolve.mjs --test
test/cap-concurrent-drain.test.js` → `# tests 1 / # pass 1 / # fail 0`
at this tip.
- `node audit/probes/probe-2773.mjs <root>` → all five probes PASS, exit
0, S3 `activeCount=1 (2 live run(s): #S3b,#S3a) at maxConcurrent 1`,
queue 1.
- `ARCH_DOCS_BASE=f2ef4dfa node core/scripts/check-arch-docs.mjs` →
`arch-docs: OK (4 changed file(s))`.
- `oxlint -c core/.oxlintrc.json plugins/workflow/src/service.js
plugins/workflow/test/tail-hang.test.js
plugins/workflow/test/cap-concurrent-drain.test.js` → 0 errors, 8
warnings, all eight pre-existing `unicorn(...)` hits on lines this diff
does not touch (321, 324, 327, 364, 704, 717, 928, 2306).
- `node .github/scripts/check-empty-catch.mjs` → exit 0 (the first tip
failed this one; see below).
- `node .github/scripts/changelog.mjs check` → 17 fragments well formed.

Not run (and why): root `npm run verify` — this fresh clone has no
`core/node_modules` and `npm ci` cannot reach a registry from this box
(the plugin suite ran with `DSH_HARNESS` pointed at the packaged DSH),
so the steps above were run individually; the core suite (no core file
changed); and the smoke bench (the diff touches no host route, turn or
delivery path).

## The first tip's `package` red, and what moved

The first tip `cfc38464` failed the `package` job at step 11 ("Tests of
the repo scripts"): `.github/scripts/check-empty-catch.test.mjs` (ThinkFlowLab#213)
listed `plugins/workflow/src/service.js:412` — the `.catch(() => {})` on
the late answer's release chain, which dropped a throw from that chain
with nothing on record. `cc35bf48` gives that handler the file's own
shape: it warns with the reason, and `releaseAll` still warns once per
Bot it could not remove. The `typeof release === 'function'` ternary in
front of `releaseAll` went with it — dead, because `releaseAll` already
answers "no release function" with an empty list. No behaviour on the
C1/C2/C4 paths changed: the three probes and the 511-case suite were
re-read at `cc35bf48` and are green above.

## Ordering fact, stated plainly

`plugins/workflow/src/service.js` carried two other open pull requests
(ThinkFlowLab#2753, ThinkFlowLab#2757) while this was prepared. Live main was re-read before
this branch was cut and was still `f2ef4dfa`, so the branch was cut from
`f2ef4dfa` itself and every reading above was taken on that base; no
rebase was needed and none was done.

## Checkpoint

<!-- flybot:checkpoint -->
```json
{
  "pr": 2773,
  "claim": "Dev-3",
  "issue": 2748,
  "summary": "The step deadline ran inside hostSerial, the same tail an unanswered host call parks for ever, so a mute host left the run running with its Bots held and its slot never freed. The expiry now runs off the tail, a start whose Bots the host does not create within 60 s is refused, and a card a drain had taken goes back to the queue. Removing the tail exposed a race: two drains in one turn both read one lane as free. A start now counts against its lane's cap until beginRun() makes the run visible. Red 0/3 at the base; three probes, the concurrent-drain case and plugins/workflow are green, 512/512.",
  "files": ["plugins/workflow/src/service.js", "plugins/workflow/test/tail-hang.test.js", "plugins/workflow/test/cap-concurrent-drain.test.js", "docs/subsystems/workflow.md", "changelog.d/2748-workflow-stuck-run.fixed.md"],
  "checks": { "ran": "cd plugins/workflow && node --import ./test/resolve.mjs --test test/tail-hang.test.js test/cap-concurrent-drain.test.js; DSH_HARNESS=<dsh> node --import ./test/resolve.mjs --test \"test/*.test.js\" (512/512); node audit/probes/probe-2773.mjs <root> (5/5 PASS); node .github/scripts/check-empty-catch.mjs; ARCH_DOCS_BASE=f2ef4dfa node core/scripts/check-arch-docs.mjs", "result": "green", "at": "a79976fddd7599ab32b0615c0595aea051305820" },
  "next": "none — ready for review"
}
```
<!-- /flybot:checkpoint -->

---------

Co-authored-by: Dev-3 <dev-3@flybot.local>
Co-authored-by: Danyang Chen <birfychen@gmail.com>

…ThinkFlowLab#2789)

Drain the lane's queue when a takeover reap ends a run (ThinkFlowLab#2787).

Fixes ThinkFlowLab#2787

Owner: Dev-5
Reviewer: Birfy
Written-by: Dev-5
Bot: Dev-5
Risk tier: T3 (path rule T2; risk-gate bumps by size — 365 changed lines
> T2's 250)
Base: 74d08c5
Claim-Id: 1f80bc23d7b74967b0fea86f03af325f

## What changed

`takeover()` — the first writable call after this window started
read-only — re-reads `workflows.json` and runs the load-time reap on
that adopted store. That reap ended the dead window's run as
`host-ended`, but unlike `reap()` it never drained `lane.queue`, so a
card parked behind that run stayed queued until some unrelated assign or
ingest. ThinkFlowLab#2757 fixed the same hole for `reap()` only.

`plugins/workflow/src/service.js` extracts `reapOnce()` = `reapRuns()`
then `hostSerial(() => drainAll())`. `takeover()` calls it, and `reap()`
returns the in-flight takeover (`takingOver ?? reapOnce()`) instead of
stacking a second drain on top — the drain stays on the serial tail, so
ThinkFlowLab#2757's cap-1 race stays closed.

`docs/subsystems/workflow.md` §3: the takeover sentence now says the
reap itself leaves `lane.queue` alone and the drain that follows the
reap starts the card, and notes a takeover reap drains like the load
reap. A changelog fragment is added.

## Red first

On the merged tree with `plugins/workflow/src/service.js` taken from
base `74d08c5d` (blob `bae4c7e6`):

```text
takeover-transition-drain.test.js   # tests 2  # pass 0  # fail 2
  not ok 1 - the takeover reap starts the card parked behind the run it ends (ThinkFlowLab#2787)
    the parked card did not start after the takeover reap ended the run
    actual [] / expected ['dead-queued']
  not ok 2 - a takeover reap drains like the load reap, and a later reap does not drain twice (ThinkFlowLab#2787)
terminal-transition-drain.test.js   # tests 8  # pass 7  # fail 1
  not ok 8 - every terminal transition drains: a takeover reap
```

`terminal-transition-drain.test.js` is the P-682 table: one bench (cap 1
plus a card parked behind the running one) and one row per terminal
transition — step done, step failed, deadline, Stop, Reset, graph edit
dropping a running node, load reap, takeover reap. Every row but the
takeover reap is already green at base, so the table pins the gap
without changing source. Injecting each drain removal turns only its own
row red (step done/failed share `afterStep`, the two reaps share
`reapOnce`); reverting only the takeover line turns just the two ThinkFlowLab#2787
tests and the table's takeover row red.

## Ran at this tip

```text
$ (cd plugins/workflow && TZ=UTC npm test)
# tests 528  # pass 528  # fail 0
$ npm_config_cache=<worktree>/.cache/npm node scripts/lint-plugins.mjs
Found 16 warnings and 0 errors.  (existing warnings)
lint-plugins: packed @flybot/workflow imported
$ node .github/scripts/changelog.mjs check
changelog: 20 fragment(s) in changelog.d/ are well formed
$ ARCH_DOCS_BASE=74d08c5dab7a6f3f60ac1cc499921941549d5c23 node core/scripts/check-arch-docs.mjs
arch-docs: OK (5 changed file(s))
```

Not run: `npm run verify` end to end and the core suite — this worktree
has no `core/node_modules`, and the change touches no `core/` file (the
diff is exactly 5 paths: `plugins/workflow/src/service.js`, its two
tests, `docs/subsystems/workflow.md`, one changelog fragment). No
`generate`: `plugins/**` is not mirrored under `patches/ds-bot/`.

## Merge note

`git merge` of live main `74d08c5d` conflicted only in
`docs/subsystems/workflow.md` §3 (main landed ThinkFlowLab#2785's sentence on the
same line); resolved by taking main's text and re-applying the ThinkFlowLab#2787
wording. `service.js` merged automatically.

## Checkpoint

<!-- flybot:checkpoint -->
```json
{
  "pr": 2789,
  "claim": "Dev-5",
  "issue": 2787,
  "summary": "A takeover reap ended the dead window's run but never drained the lane's queue, so a card parked behind it waited for an unrelated assign. reapOnce() now reaps and then drains on the serial tail; takeover() uses it and reap() returns the in-flight takeover instead of stacking a second drain. A table test pins that every terminal transition drains, and the subsystem doc says the reap leaves the queue alone and the following drain starts the card.",
  "files": [
    "plugins/workflow/src/service.js",
    "plugins/workflow/test/takeover-transition-drain.test.js",
    "plugins/workflow/test/terminal-transition-drain.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/2787-takeover-drain.fixed.md"
  ],
  "checks": {
    "ran": "(cd plugins/workflow && TZ=UTC npm test)",
    "result": "green",
    "at": "b9c3735e8570aef8d523d8ae786c68e0a1fd4a2e"
  },
  "next": "none — ready for review"
}
```
<!-- /flybot:checkpoint -->